Skip to content

norm stats bound check#285

Open
aidang3019 wants to merge 1 commit intoaniketh/bad-idx-fallbackfrom
aidan/bounds-check
Open

norm stats bound check#285
aidang3019 wants to merge 1 commit intoaniketh/bad-idx-fallbackfrom
aidan/bounds-check

Conversation

@aidang3019
Copy link
Contributor

No description provided.

@aidang3019 aidang3019 marked this pull request as ready for review March 13, 2026 16:07
Copy link
Contributor Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.


return data

def set_norm_stats(self, norm_stats: dict) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed? I thought when we instantiate ZarrDataset we pass in the norm stats?

)
return (next_idx, origin, attempts)

def _get_image_fallback_idx(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does get_image_fallback_idx have diff logic from get_fallback_idx

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i thought we didnt want to random resample frames, but just do closest clean frame

@@ -798,6 +967,7 @@ def get_item_keys(
idx: int,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Re: line +965]

This function isn't being used so all these changes can be deleted, right?

See this comment inline on Graphite.

),
)

# Propagate norm stats to all zarrdatasets out of bounds check in getitem
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason we can't just directly set dataset.norm_stats = data_schematic.norm_stats. I wish we didn't have to add norm stats as an attribute after creating the dataset, but I'm not sure if there's a clean pattern to avoid this

Copy link
Collaborator

@SimarKareer SimarKareer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants